Allow Clients with Optional Client Auth to not negotiate Client Auth #816
Conversation
alexw91
force-pushed
the
optionalClientAuth
branch
from
b8dcf19
to
4497f57
Compare
tls/s2n_handshake_io.c
Outdated
| } | ||
|
|
||
| /* If server, and Client Auth is REQUIRED or OPTIONAL, then the server must send the CLIENT_CERT_REQ Message*/ | ||
| if (conn->mode == S2N_SERVER && client_cert_auth_type != S2N_CERT_AUTH_NONE) { |
There was a problem hiding this comment.
else if?
S2N_SERVER and S2N_CLIENT are mutually exclusive, so make this an else?
alexw91
force-pushed
the
optionalClientAuth
branch
from
4497f57
to
c75c18e
Compare
|
|
||
| if (conn->mode == S2N_CLIENT && client_cert_auth_type == S2N_CERT_AUTH_REQUIRED) { | ||
| /* If we're a client, and Client Auth is REQUIRED, then the Client must expect the CLIENT_CERT_REQ Message */ | ||
| conn->handshake.handshake_type |= CLIENT_AUTH; |
There was a problem hiding this comment.
Since the client is dynamically reacting to the presence of a CertificateRequest now, do we still need to have the client explicitly setting a client auth mode?
There was a problem hiding this comment.
We're still honoring the client_cert_auth_type value.
If it's NONE we'll abort the handshake rather than enter the ClientCertRequest state, if OPTIONAL we'll enter that state only if the Server asks and potentially send an empty response if no ClientCert is configured, and if REQUIRED we'll always expect the ClientCertRequest from the server and abort the handshake if no ClientCert is configured.
There was a problem hiding this comment.
Sorry, I think my comment was a little confusing. I meant to ask if we could relax the constraint of REQUIRED altogether. Could the client just dynamically react to the CertificateRequest in all cases (REQUIRED, OPTIONAL, and NONE)? If it has a client cert configured and it receives a CertificateRequest, send the cert. If it does not have a client cert configured and it receives a CertificateRequest, send an empty response. If no CertificateRequest is sent, carry on.
If the client is configured to REQUIRED and the server does not send the CertificateRequest, why fail the handshake? Why not just let the server decide the requirements of client authentication? Maybe I'm missing a use case or complexity here
There was a problem hiding this comment.
I think there are clients out there that would expect to fail if the server doesnt support mutual auth. by allowing it to succeed and having the server decide what to do, you are potentially allowing your client to start communicating with an untrusted or unexpected server.
There was a problem hiding this comment.
In that case, wouldn't the client still have authenticated the server to ensure it was communicating with a trusted endpoint? The server would have still sent it's own certificate. The client could enforce server auth without having to enforce mutual auth.
There was a problem hiding this comment.
I think this is an API design discussion. If a developer configures their s2n client with CERT_AUTH_NONE, what are they trying to say?
I see two plausible options:
- "I don't want to connect to any servers that request Client Auth, abort the handshake if a Client Cert is requested"
- "I never want to send any Client Certs, just send an empty ClientCert message if necessary, and let the server decide to abort the handshake if necessary"
The 2nd option is better for compatibility reasons, since the client can connect to more Servers (those that always request a ClientCert but don't require it), but the 2nd option has exactly the same behavior as an s2n Client with no ClientCert configured with CERT_AUTH_OPTIONAL (and it seems weird for someone to add a ClientCert but set ClientCertAuth to NONE).
If we adopt the 2nd option, the behavior of the 1st option can also be replicated manually by calling s2n_connection_client_cert_used() after the handshake is completed, and aborting the handshake if necessary, but that requires more work on the developer's part.
I'm okay going either way.
There was a problem hiding this comment.
The idea of optional client certs doesn't map to the client very well. The RFC says:
If no suitable certificate is available, the client MUST send a certificate message containing no certificates.
...
If the client does not send any certificates, the server MAY at its discretion either continue the handshake without client authentication, or respond with a fatal handshake_failure alert.
I interpret that as basically saying the RFC doesn't even really allow for clients deciding to end the handshake based on the existence (or lack of) a certificate request. The rules for either party of a handshake sending an error are a little fuzzy in that you could interpret it to mean that if an implementation decides something is an error then it can bail-out, but I interpret it more that if an error as defined by the RFC is encountered then bail out. Since the client certificate portion doesn't allow the client to determine that there's been an error, it wouldn't be valid for the client to bail-out here.
All that is to say that per the RFC, I think setting any of the REQUIRED/OPTIONAL/NONE in a client context is invalid. If the consumer, at the application level, wants to require the use of a client cert then s2n_connection_client_cert_used() is the mechanism for allowing that (after the handshake is complete).
If you want a feature that allows the client to bail out of connections based on the server's client certificate behavior, I think there's probably a few use-cases for that and it wouldn't be a big deal in practice, but I don't think it would be compliant with the RFC.
There was a problem hiding this comment.
Opened #819 to continue this discussion.
Proof fix and small README fix for SAW
Issue # (if available): #815
Description of changes:
Fix Client side bug in handling Optional Client Auth.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.